-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a replay analysis overlay #27334
Conversation
committing early version for others in the discussion to do their own testing with it
Hit & aim markers are skinnable. Hidden can be toggled off. Aim line with skinnable color was added. The fadeout time is based on the approach rate. Cursor hide fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not reall doing a full review just obvious issues
osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs
Outdated
Show resolved
Hide resolved
Uses pooling for all analysis objects and creates the lifetime entries from replay data when the analysis container is constructed.
improve default design
better to add skinnable elements later
Just going to boil it down to the main components, so skinning can be added later. |
You should probably break this PR up into different components instead of just having 1 big PR that is essentially adding "Rewind" to replays. |
I think the only part where it would make sense to split up is between the base analysis settings implementation and the osu!std analysis implementation, but I don't know that it's really necessary to do that. |
maybe I'm biased from circleguard but I think crosses look better than plusses for hit markers, going off the linked video. afaik rewind does crosses as well |
I initially did a cross design to match rewind, but I think the difference in using a plus is negligible. As peppy mentioned in his review, it could be confusing to use a cross since that design represents a miss. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another very general pass..
Disagree, it's useful to see the different keys for identifying issues such as a weaker finger |
(haven't built myself, going off @dani211e's video (thanks!)) -
|
@Sheppsu one other issue i'm not going to fix in this PR is that rewinding draws previous frames in front of newer ones rather than behind. maybe something you can add to the todo list. |
Ah ok, I'll look into that for a follow-up. |
Also show both mouse buttons at once on frame markers.
c77f1dc
to
167e3a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix seems fine, thanks. |
Design is based upon Rewind and discussion #26856.
Adds:
OsuAnalysisContainer
Video showing it with different toggles applied: https://media.sheppsu.me/view?n=gPqsCeBmyrw (video was too big to attach :P )